HideTracker: Guard against accessing invalid pointer#2860
Conversation
|
Yes, that seems to fix it. Could you also add the same safeguard before this line gala/src/ShellClients/HideTracker.vala Line 58 in f183e16 because even with this patch applied, sometimes I get this backtrace Core was generated by `/usr/bin/gala'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0 0x000055e6f519bb41 in gala_positioned_window_get_window (self=0x55e7356c72f0) at ../src/ShellClients/PositionedWindow.vala:9
9 public Meta.Window window { get; construct; }
[Current thread is 1 (Thread 0x7feff60c1fc0 (LWP 3699))]
(gdb) bt
#0 0x000055e6f519bb41 in gala_positioned_window_get_window (self=Invalid quark 4213827520
0x55e7356c72f0) at ../src/ShellClients/PositionedWindow.vala:9
#1 _gala_hide_tracker___lambda41_ (self=0x55e73620ead0 [GalaHideTracker]) at ../src/ShellClients/HideTracker.vala:60
#2 __gala_hide_tracker___lambda41__meta_cursor_tracker_position_invalidated
(_sender=0x55e7354d60a0 [MetaCursorTracker], self=0x55e73620ead0) at ../src/ShellClients/HideTracker.vala:59
#7 0x00007feffb1d8d33 in <emit signal 'position-invalidated' on instance 0x55e7354d60a0 [MetaCursorTracker]>
(instance=instance@entry=0x55e7354d60a0, signal_id=<optimized out>, detail=detail@entry=0) at ../gobject/gsignal.c:3624
#3 0x00007feffb1b7e7c in g_closure_invoke
(closure=0x55e7362ac970, return_value=0x0, n_param_values=1, param_values=0x7ffdae6184c0, invocation_hint=0x7ffdae618410)
at ../gobject/gclosure.c:916
#4 0x00007feffb1d6809 in signal_emit_unlocked_R
(node=node@entry=0x7ffdae6185e0, detail=detail@entry=0, instance=instance@entry=0x55e7354d60a0, emission_return=emission_return@entry=0x0, instance_and_params=instance_and_params@entry=0x7ffdae6184c0) at ../gobject/gsignal.c:3929
#5 0x00007feffb1d89e8 in signal_emit_valist_unlocked
(instance=instance@entry=0x55e7354d60a0, signal_id=signal_id@entry=206, detail=detail@entry=0, var_args=var_args@entry=0x7ffdae618740)
at ../gobject/gsignal.c:3561
#6 0x00007feffb1d8c78 in g_signal_emit_valist
(instance=0x55e7354d60a0, signal_id=206, detail=0, var_args=var_args@entry=0x7ffdae618740) at ../gobject/gsignal.c:3304
#8 0x00007feff9c4e76c in meta_cursor_tracker_invalidate_position (tracker=0x55e7354d60a0 [MetaCursorTracker])
at ../src/backends/meta-cursor-tracker.c:403
#9 meta_backend_update_from_event (backend=0x55e733310810 [MetaBackendNative], event=0x7fef380edb70)
at ../src/backends/meta-backend.c:2050
#10 meta_backend_update_from_event (backend=0x55e733310810 [MetaBackendNative], event=0x7fef380edb70)
at ../src/backends/meta-backend.c:2027
#11 dispatch_clutter_event (backend=0x55e733310810 [MetaBackendNative]) at ../src/backends/meta-backend.c:1133
#12 0x00007feff9c4e8f1 in clutter_source_dispatch (source=0x55e73351e590, callback=<optimized out>, user_data=<optimized out>)
at ../src/backends/meta-backend.c:1176
#13 0x00007feffb092f24 in g_main_dispatch (context=0x55e733306fc0) at ../glib/gmain.c:3591
#14 g_main_context_dispatch_unlocked (context=0x55e733306fc0) at ../glib/gmain.c:4451
#15 0x00007feffb097038 in g_main_context_iterate_unlocked
(context=0x55e733306fc0, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>) at ../glib/gmain.c:4516
#16 0x00007feffb0972e7 in g_main_loop_run (loop=0x55e7356c6d60) at ../glib/gmain.c:4721
#17 0x00007feff9cb546a in meta_context_run_main_loop (context=<optimized out>, error=error@entry=0x7ffdae618a48)
at ../src/core/meta-context.c:506
#18 0x000055e6f516729f in gala_main (args=<optimized out>, args_length1=<optimized out>) at ../src/Main.vala:92
#19 0x00007feff94f5681 in __libc_start_call_main (main=main@entry=0x55e6f5151660 <main>, argc=argc@entry=1, argv=argv@entry=0x7ffdae618df8)
at ../sysdeps/nptl/libc_start_call_main.h:59
#20 0x00007feff94f5798 in __libc_start_main_impl
(main=0x55e6f5151660 <main>, argc=1, argv=0x7ffdae618df8, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, stack_e
nd=0x7ffdae618de8) at ../csu/libc-start.c:360
#21 0x000055e6f51516a5 in _start () |
lenemter
left a comment
There was a problem hiding this comment.
Why do we need window_unmanaged variable? Can't we just check for panel != null?
28cf95e to
d984d91
Compare
Unfortunately not because valas weak and unowned references aren't "real" weak references as you would for example obtain via Object.weak_ref. They are in fact just pointers without any extra guards and don't get automatically set to NULL if that what they point to gets freed. This means that after the panel is freed the panel reference in hide tracker is not null but still a real pointer however the memory region it points to is invalid now. But we can of course set the panel to null instead of introducing a window_unmanaged variable, idk why I didn't think of that. That also makes sure that we definitely don't have a dangling pointer so thanks for mentioning that :) @garaevdi I also added the second guard if you want to try it again |
|
It gets a little bit further, but still crashes later🫠️ Core was generated by `/usr/bin/gala'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0 meta_window_has_focus (window=0x0) at ../src/core/window.c:6692
6692 return window->has_focus;
[Current thread is 1 (Thread 0x7fae4dd81fc0 (LWP 2553))]
(gdb) bt
#0 meta_window_has_focus (window=0x0) at ../src/core/window.c:6692
#1 0x0000558b544125c7 in gala_hide_tracker_check_trigger_conditions (self=<optimized out>)
at ../src/ShellClients/HideTracker.vala:139
#6 0x00007fae52534d33 in <emit signal 'notify:focus-window' on instance 0x558b64e427d0 [MetaDisplay]>
(instance=<optimized out>, signal_id=<optimized out>, detail=<optimized out>)
at ../gobject/gsignal.c:3624
#2 0x00007fae52513e7c in g_closure_invoke
(closure=0x558b66155170, return_value=0x0, n_param_values=2, param_values=0x7ffc9b19d960, invocation_hint=0x7ffc9b19d8b0) at ../gobject/gclosure.c:916
#3 0x00007fae52532809 in signal_emit_unlocked_R
(node=node@entry=0x7ffc9b19daa0, detail=detail@entry=1245, instance=instance@entry=0x558b64e427d0, emission_return=emission_return@entry=0x0, instance_and_params=instance_and_params@entry=0x7ffc9b19d960)
at ../gobject/gsignal.c:3929
#4 0x00007fae525349e8 in signal_emit_valist_unlocked
(instance=instance@entry=0x558b64e427d0, signal_id=signal_id@entry=1, detail=detail@entry=1245, var_args=var_args@entry=0x7ffc9b19dc00) at ../gobject/gsignal.c:3561
#5 0x00007fae52534c78 in g_signal_emit_valist
(instance=0x558b64e427d0, signal_id=1, detail=1245, var_args=var_args@entry=0x7ffc9b19dc00)
at ../gobject/gsignal.c:3304
#7 0x00007fae525272b4 in g_object_dispatch_properties_changed
(object=<optimized out>, n_pspecs=<optimized out>, pspecs=<optimized out>) at ../gobject/gobject.c:1894
#8 g_object_notify_by_spec_internal (object=0x558b64e427d0 [MetaDisplay], pspec=<optimized out>)
at ../gobject/gobject.c:1999
#9 g_object_notify (object=0x558b64e427d0 [MetaDisplay], property_name=0x7fae511fe533 "focus-window")
at ../gobject/gobject.c:2049
#10 g_object_notify
(object=object@entry=0x558b64e427d0 [MetaDisplay], property_name=property_name@entry=0x7fae511fe533 "focus-window") at ../gobject/gobject.c:2026
#11 0x00007fae5109e1d9 in meta_display_update_focus_window (display=0x558b64e427d0 [MetaDisplay], window=0x0)
at ../src/core/display.c:1345
#12 meta_display_set_input_focus
(display=0x558b64e427d0 [MetaDisplay], window=0x0, timestamp=<optimized out>)
at ../src/core/display.c:1387
#13 0x00007fae510c9d04 in meta_window_unmanage
(window=0x558b65eefa80 [MetaWindowWayland], timestamp=<optimized out>) at ../src/core/window.c:1619
#14 0x00007fae5114b478 in meta_wayland_shell_surface_destroy_window (shell_surface=<optimized out>)
at ../src/wayland/meta-wayland-shell-surface.c:344
#15 0x00007fae5115061d in xdg_toplevel_destructor (resource=<optimized out>)
at ../src/wayland/meta-wayland-xdg-shell.c:221
#16 0x00007fae512f3bd0 in remove_and_destroy_resource (element=0x558b65d4bc00, data=<optimized out>, flags=0)
at ../src/wayland-server.c:796
#17 0x00007fae50780056 in ffi_call_unix64 () at ../src/x86/unix64.S:104
#18 0x00007fae5077c05c in ffi_call_int
(cif=cif@entry=0x7ffc9b19e040, fn=fn@entry=0x7fae5114eda0 <xdg_toplevel_destroy.lto_priv>, rvalue=<optimized out>, rvalue@entry=0x0, avalue=avalue@entry=0x7ffc9b19e110, closure=closure@entry=0x0)
at ../src/x86/ffi64.c:676
#19 0x00007fae5077ed8e in ffi_call
(cif=cif@entry=0x7ffc9b19e040, fn=0x7fae5114eda0 <xdg_toplevel_destroy.lto_priv>, rvalue=rvalue@entry=0x0, avalue=avalue@entry=0x7ffc9b19e110) at ../src/x86/ffi64.c:713
#20 0x00007fae512ef510 in wl_closure_invoke (closure=closure@entry=0x558b664ebbb0, target=<optimized out>,
target@entry=0x558b65d4bc00, opcode=opcode@entry=0, data=<optimized out>,
data@entry=0x558b65ce7a10, flags=2) at ../src/connection.c:1241
#21 0x00007fae512f4d88 in wl_client_connection_data
(fd=<optimized out>, mask=<optimized out>, data=<optimized out>) at ../src/wayland-server.c:483
#22 0x00007fae512f3262 in wl_event_loop_dispatch (loop=0x558b62e2d3f0, timeout=<optimized out>,
timeout@entry=0) at ../src/event-loop.c:1059
#23 0x00007fae5112771b in wayland_event_source_dispatch
(base=<optimized out>, callback=<optimized out>, data=<optimized out>)
at ../src/wayland/meta-wayland.c:150
#24 0x00007fae523eef24 in g_main_dispatch (context=0x558b62c21fc0) at ../glib/gmain.c:3591
#25 g_main_context_dispatch_unlocked (context=0x558b62c21fc0) at ../glib/gmain.c:4451
#26 0x00007fae523f3038 in g_main_context_iterate_unlocked
(context=0x558b62c21fc0, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>)
at ../glib/gmain.c:4516
#27 0x00007fae523f32e7 in g_main_loop_run (loop=0x558b64fdbbb0) at ../glib/gmain.c:4721
#28 0x00007fae510b546a in meta_context_run_main_loop
(context=<optimized out>, error=error@entry=0x7ffc9b19e788) at ../src/core/meta-context.c:506
#29 0x0000558b543de29f in gala_main (args=<optimized out>, args_length1=<optimized out>)
at ../src/Main.vala:92
#30 0x00007fae50855681 in __libc_start_call_main
(main=main@entry=0x558b543c8660 <main>, argc=argc@entry=1, argv=argv@entry=0x7ffc9b19eb38)
at ../sysdeps/nptl/libc_start_call_main.h:59
#31 0x00007fae50855798 in __libc_start_main_impl
(main=0x558b543c8660 <main>, argc=1, argv=0x7ffc9b19eb38, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7ffc9b19eb28) at ../csu/libc-start.c:360
#32 0x0000558b543c86a5 in _start () |
d984d91 to
be633f8
Compare
|
@garaevdi I added some more guards and I think I got all places where the panel is accessed if you want to try it again 😅 |
|
Yep, this seems to fix the crash. The only issue is that after a few client "crashes" pressure barrier stops working, but I couldn't find a reliable way to reproduce this. |
Fixes #2859
@garaevdi could you try if this fixes the crash for you?
Because I think your backtrace points more towards
gala/src/ShellClients/HideTracker.vala
Line 112 in f183e16
Also I think that not positionedwindow.window is null but rather the positionedwindow itself was already freed because the hidetracker only holds an unowned reference on it. The HideTracker however isn't freed because it is captured by the wait for window actor callback.
With this PR we guard against accessing that reference if the window was unmanaged.
For now I hope this should fix it but we had this problem in the past a few times so I think I'll follow up some time by just passing the meta.window and the anchor instead of the whole panelwindow to avoid having to hold an unowned reference.